[TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector #5039

samsonasik · 2023-09-17T23:05:03Z

@staabm this is to handle fallback from param on return new object:

final class FallbackFromParam
{
    public function action(SomeResponse $someResponse)
    {
        if (rand(0, 1)) {
            return new SomeResponse();
        }

        return $someResponse;
    }
}

…ewRector

samsonasik · 2023-09-17T23:06:26Z

implemented 🎉

samsonasik · 2023-09-17T23:19:09Z

@TomasVotruba the phpstan notice seems due to new phpstan-extensions release that cause invalid check on realpath() which can be false:

------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  packages/Caching/Detector/ChangedFilesDetector.php:90
 ------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  - '#Strict comparison using \=\=\= between string and false will always evaluate to false#'
 ------------------------------------------------------------------------------------------------------------------------------------------------------------------------

see symplify/phpstan-extensions@b2b82e1#r127599628

samsonasik · 2023-09-17T23:28:21Z

@TomasVotruba I created PR to symplify/phpstan-extensions to revert realpath:

Remove realpath on NativeFunctionDynamicFunctionReturnTypeExtension symplify/phpstan-extensions#7

staabm · 2023-09-18T05:18:59Z

...on/Rector/ClassMethod/ReturnTypeFromReturnNewRector/Fixture/fallback_from_param_self.php.inc

+
+final class FallbackFromParamSelf
+{
+    public function action(self $obj): \Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Fixture\FallbackFromParamSelf


Do we need another test for a static and/or parent typed param?

static already error on the first place :)

https://3v4l.org/cEZik

multiple return already skipped at

rector-src/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromReturnNewRector.php

Lines 185 to 186 in c956f86

$returnType = $this->returnTypeInferer->inferFunctionLike($node);

if ($returnType instanceof UnionType) {

samsonasik · 2023-09-18T06:00:56Z

I updated to use @var string|false and check with more verbose is_dir() && is_file() in various locations based on @TomasVotruba suggestion at

symplify/phpstan-extensions@b2b82e1#r127599628

samsonasik · 2023-09-18T06:02:37Z

All checks have passed 🎉 @TomasVotruba I think it is ready.

staabm · 2023-09-18T06:04:09Z

packages/Config/RectorConfig.php

+            // not directory
+            && ! is_dir($skipRule)
+            // not file
+            && ! is_file($skipRule)
            // a Rector end
            && str_ends_with($skipRule, 'Rector')


We should do this cheap chrck earlier

updated 8fae378

samsonasik · 2023-09-18T06:12:33Z

All checks have passed 🎉 @TomasVotruba I am merging it to have faster feedback to test ;)

TomasVotruba · 2023-09-18T06:15:19Z

utils/Command/MissingInSetCommand.php

+            /** @var string|false $setRealpath */
+            $setRealpath = realpath($setFile);
+            $setRealpath = (string) $setRealpath;
            $relativeFilePath = Strings::after($setRealpath, getcwd() . '/');


This will be always string, as we use existing set paths:

$setRealpath = realpath($setFile); $relativeFilePath = Strings::after($setRealpath, getcwd() . '/');

sure, I updated at :

Use realpath() as string on MissingInSetCommand #5040

Thanks 👏

the php process might still not be able to read this file (e.g. because of filesystem persmissions) and return false?

@staabm that's what I thought, but since tools/ directory is not shipped to rector/rector, I guess it safe for user, the concern is probably when directory/set removed in the future and not detected.

The better solution is probably use:

Assert::string($setRealpath);

so it will be detected early when path removed/renamed.

staabm · 2023-09-18T06:16:42Z

Thanks 🙏

[TypeDeclaration] Handle fallback from param on ReturnTypeFromReturnN…

be8b2eb

…ewRector

samsonasik requested a review from TomasVotruba as a code owner September 17, 2023 23:05

implemented 🎉

7594001

actions-user and others added 3 commits September 17, 2023 23:07

[ci-review] Rector Rectify

2a154c6

rename fixture

e69bc6e

rename fixture

f93af9d

samsonasik changed the title ~~[TypeDeclaration] Handle fallback from param on ReturnTypeFromReturnNewRector~~ [TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector Sep 17, 2023

samsonasik added 2 commits September 18, 2023 06:10

fix phpstan

ed8860f

fix phpstan

11c285c

staabm reviewed Sep 18, 2023

View reviewed changes

samsonasik added 2 commits September 18, 2023 12:46

realpath() always string after file_exists() on bin/rector.php

402d4ab

Fix phpstan

3bb67a3

staabm reviewed Sep 18, 2023

View reviewed changes

samsonasik added 2 commits September 18, 2023 13:05

cheap check early

8fae378

Trigger CI

a5c966e

samsonasik merged commit 2cf8d83 into main Sep 18, 2023
39 checks passed

samsonasik deleted the fallback-param branch September 18, 2023 06:12

TomasVotruba reviewed Sep 18, 2023

View reviewed changes

samsonasik mentioned this pull request Sep 18, 2023

Use realpath() as string on MissingInSetCommand #5040

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector #5039

[TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector #5039

samsonasik commented Sep 17, 2023

samsonasik commented Sep 17, 2023

samsonasik commented Sep 17, 2023

samsonasik commented Sep 17, 2023

staabm Sep 18, 2023

samsonasik Sep 18, 2023

samsonasik Sep 18, 2023

samsonasik commented Sep 18, 2023

samsonasik commented Sep 18, 2023

staabm Sep 18, 2023

samsonasik Sep 18, 2023 •

edited

Loading

samsonasik commented Sep 18, 2023

TomasVotruba Sep 18, 2023 •

edited

Loading

samsonasik Sep 18, 2023

TomasVotruba Sep 18, 2023

staabm Sep 18, 2023

samsonasik Sep 18, 2023

staabm commented Sep 18, 2023

	$returnType = $this->returnTypeInferer->inferFunctionLike($node);
	if ($returnType instanceof UnionType) {

[TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector #5039

[TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector #5039

Conversation

samsonasik commented Sep 17, 2023

samsonasik commented Sep 17, 2023

samsonasik commented Sep 17, 2023

samsonasik commented Sep 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samsonasik commented Sep 18, 2023

samsonasik commented Sep 18, 2023

Choose a reason for hiding this comment

samsonasik Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

samsonasik commented Sep 18, 2023

TomasVotruba Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staabm commented Sep 18, 2023

samsonasik Sep 18, 2023 •

edited

Loading

TomasVotruba Sep 18, 2023 •

edited

Loading